Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

enable building the cblas64 library #460

Merged
merged 1 commit into from
Oct 31, 2020
Merged

enable building the cblas64 library #460

merged 1 commit into from
Oct 31, 2020

Conversation

epsilon-0
Copy link
Contributor

changes all function signatures to use CBLAS_INDEX
also fixes internal variables to be CBLAS_INDEX
all tests are passing

A lot of the optimized implementations are providing an implementation for CBLAS64
It is good to have the reference implementation also providing this, as we already have the 64 bit libraries for others.

I've followed your standard syntax rules from other files.
The variable name used is the standard CBLAS_INDEX which should keep it compatible with all other files.

Would be really awesome to finally have this reference implementation ❤️

Thanks a lot,
Aisha

@epsilon-0
enable building the cblas64 library
4177968 
changes all function signatures to use CBLAS_INDEX
also fixes internal variables to be CBLAS_INDEX
all tests are passing
@langou
Copy link
Contributor

langou commented Oct 27, 2020

I am in favor of this PR.

@codecov
Copy link

codecov bot commented Oct 27, 2020 •
edited
Loading

Codecov Report

Merging #460 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #460   +/-   ##
=======================================
  Coverage   81.86%   81.86%           
=======================================
  Files        1863     1863           
  Lines      181096   181096           
=======================================
  Hits       148263   148263           
  Misses      32833    32833

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fbb4f8...4177968. Read the comment docs.

@martin-frbg
Copy link
Collaborator

I'm certainly not against it either, even if it means losing one competitive advantage of OpenBLAS 😃
Just for cross-referencing: this is expected to fix both #459 and #184

@epsilon-0
Copy link
Contributor Author

I'm certainly not against it either, even if it means losing one competitive advantage of OpenBLAS smiley
Just for cross-referencing: this is expected to fix both #459 and #184

XD
I'd say this is not going to make OpenBLAS lose an advantage.
In fact, I'd say this should actually be a positive note for OpenBLAS (and any other provider, like blis), because now it can even say that it provides an optimized reference CBLAS64 API, as opposed to providing a CBLAS64 API, as previously we did not have a standardized CBLAS64 API.

@langou langou merged commit a6870da into Reference-LAPACK:master Oct 31, 2020
@epsilon-0
Copy link
Contributor Author

@langou
thanks a lot for the quick merge!

christoph-conrads pushed a commit to christoph-conrads/lapack that referenced this pull request May 23, 2021
@langou
Merge pull request Reference-LAPACK#460 from epsilon-0/master
ba4c63f 
enable building the cblas64 library
@weslleyspereira weslleyspereira mentioned this pull request Apr 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@epsilon-0 @langou @martin-frbg